Skip to content

ARROW-14807: [R] Implement bindings for lubridate am and pm#12244

Closed
djnavarro wants to merge 4 commits into
apache:masterfrom
djnavarro:ARROW-14807
Closed

ARROW-14807: [R] Implement bindings for lubridate am and pm#12244
djnavarro wants to merge 4 commits into
apache:masterfrom
djnavarro:ARROW-14807

Conversation

@djnavarro

@djnavarro djnavarro commented Jan 24, 2022

Copy link
Copy Markdown
Contributor

I think this exactly mirrors the lubridate equivalents

@github-actions

github-actions Bot commented Jan 24, 2022

Copy link
Copy Markdown

https://issues.apache.org/jira/browse/ARROW-14807

@github-actions

github-actions Bot commented Jan 24, 2022

Copy link
Copy Markdown

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

thisisnic
thisisnic approved these changes Jan 25, 2022
View reviewed changes

@thisisnic thisisnic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR, and for flagging up the lubridate code comparison - looks great!

thisisnic
thisisnic requested changes Jan 25, 2022
View reviewed changes

@thisisnic thisisnic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, since approving this the CI completed with some Windows failures; mind updating these accordingly?

@djnavarro

djnavarro commented Jan 25, 2022

Copy link
Copy Markdown
Contributor Author

Yep, no worries! I'll look into this tomorrow (it's a public holiday here today)

@djnavarro

djnavarro commented Jan 27, 2022

Copy link
Copy Markdown
Contributor Author

@thisisnic: should be fixed now. as usual, I forgot to skip the timezone-aware test on windows 🙂

thisisnic
thisisnic approved these changes Jan 27, 2022
View reviewed changes

@thisisnic thisisnic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff!

@thisisnic thisisnic closed this in 79800d4 Jan 27, 2022
@ursabot

ursabot commented Jan 27, 2022 •
edited
Loading

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = 01855c7 and contender = 79800d4. 79800d4 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️2.86% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.22% ⬆️0.0%] ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thisisnic thisisnic thisisnic approved these changes

Assignees

No one assigned

Labels

Component: R

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@djnavarro @ursabot @thisisnic